Add quick flag and use unsigned unlifted Word##198
Conversation
| where | ||
| byte = toBitString (Bits' 8) | ||
| bcWrapperMagicConst :: Word | ||
| bcWrapperMagicConst = 0x0b16c0de |
There was a problem hiding this comment.
Wait, did this code change? The previous version used 17, whereas this version uses 16.
| -- Verify a Word is 64-bits (at compile time) | ||
| $(return $ if isTrue# ((int2Word# 3#) `eqWord#` | ||
| (((int2Word# 0xF0#) `uncheckedShiftL#` 58#) | ||
| `uncheckedShiftRL#` 62#)) | ||
| then [] | ||
| else error "Word type must be 64-bits!" | ||
| ) |
There was a problem hiding this comment.
I'm very leery about making this library completely unbuildable on 32-bit architectures. Why not just use Word64 instead?
| {-# LANGUAGE CPP #-} | ||
| {-# LANGUAGE MagicHash #-} | ||
| {-# LANGUAGE PatternSynonyms #-} | ||
| {-# LANGUAGE QuasiQuotes #-} |
There was a problem hiding this comment.
As far as I can tell, the QuasiQuotes extension is never used anywhere in this module, just TemplateHaskell.
| #ifdef QUICK | ||
| fromBitString (BitString _ i) = x | ||
| #else |
There was a problem hiding this comment.
I'm not a fan of sprinkling CPP everywhere throughout the library like this, since (1) it makes adds a lot of lexical noise, and (2) it makes it more difficult to remember to keep the two versions of every function in sync, since you have to remember to build with both -fquick and -f-quick unless you want to risk bitrot.
Rather this going with this approach, why not do something like this instead?
fromBitString :: (Num a, Bits a) => BitString -> a
fromBitString (BitString l i)
| quickMode = x
| otherwise =
case bitSizeMaybe x of
...Where:
quickMode :: Bool
#if defined(QUICK)
quickMode = True
#else
quickMode = False
#endifThis way, we can continue to typecheck both versions of each function, decreasing the chances of bitrot. Since quickMode will statically be known to be True or False, the optimizer will be able to make swift work of it.
| -> ByteString {-^ the ByteString to extract from -} | ||
| -> Either String (() -> (# Int#, Int# #)) | ||
| -> Either String (() -> (# Word#, Int# #)) | ||
| extractFromByteString !bitLim# !sBit# !nbits# bs = |
There was a problem hiding this comment.
Now that we have a quick flag, is it worth making extractFromByteString' above the quick version of this function?
| #ifndef QUICK | ||
| Assert.recordSizeGreater r (1 + asmStrSize) | ||
| #endif |
There was a problem hiding this comment.
Similarly, rather than surrounding every assertion with CPP, it would be better to redefine each assertion function in terms of quickMode. Something like:
recordSizeGreater x y = Control.Monad.unless quickMode $ ...2 participants
This makes further performance modifications to use unsigned
Word#instead of signedInt#and adds aquickflag (disabled by default). When thequickflag is enabled, much of the error detection, verification, and other internal checks are removed; this mode is a further incremental performance gain but it should only be used in situations where well-formed bitcode from known-supported versions of LLVM willl be used.